Understanding OpenBazaar p2p networking

gistfile1.txt

@hoffmabc i hope you have the patience to read this, at least it'll be a nice trip back
through the code, a refresher, or perhaps you can clear any missunderstandings I have 
made as I read the p2p code.

I see the following chain of events:

from the Genesis, start_node()  (tornadoloop.py) may or may not have a list of peers to 
boostrap the p2p connections, when it creates MarketApplication (still in tornadoloop.py), 
which is the one listening.

When MarketApplication starts it has a few important responsabilities, that maybe 
should be or should not be here, not important now, it creates one important class
right after it gets a DB connection, self.transport.

(1) This CryptoTransportLayer object I see is the one responsible for handling the
messages that Application will be listening on after it will call that listen()
while in start_node()... hmm
[This object will be the center of my interest a few paragraphs from now]

the Market() object is created, I suppose this is where all the "business logic is created", 
the store, the contracts, the orders, etc... 

and after that we tell our CyrptoTransportLayer object to "join_network", 
a very important method.  (a method I just saw you recently removed from being called 
too much with something that looked like a Timer, should probably happen only once 
if you're able to bootstrap that is...)

Join network means basically adding those peer objects... to the dht..., 
again no limits set to the number of peers that can be added, the dht will explode 
if enough peers are added, it will just keep lumping them into k-sized buckets until 
it runs out of memory... unless there's some code down there to avoid that from happening.

So up here's where the weird architectural shenanigans happen, these objects 
(dht and the layer above, we must see dht as a neutral distributed container, 
this way, if this implementation is not the best, we can reuse an existing one, 
say bittorrent's which already has millions of nodes to support us, and it is 
content agnostic) should be more independent of each other.

Inside dht.add_peer, there's a call to (Crypto) transport.get_crypto_peer, 
which looks like a CryptoPeerConnection() factory method. So inside the dht(), 
we're looking for specifics, doesn't sound like a general purpose dht object, hmm.

the dht?

I think the dht ought to be a more general purpose container, its contents just happen 
to be distributed among many computers, it should just ask for a Connection object, 
we should make Connection an abstract class, and have the dht not know about the 
implementation of other objects, at this point it's explictly asking for a kind 
of connection object, and that makes us inflexible... and ultimately... why is it asking 
for a CryptoPeerConnection object, it doesn't handle it's own connections internally? 
(I'm curious about this dht implementation and how it's been hacked)

As soon as those connections are instantiated they try to connect to a socket with very 
little time out (if those are miliseconds), also if it's just 1 second, it might 
be too low if this is a peer connection, peers might take a while to reach, specially 
if you have a global network with countries with shitty connections. 
So this method I think might be giving a lot of False negatives.

So if this check_port() method gets a response it will will sends the famous 'hello' mesg,
so this is the beginning of our handshake. we send our publick key, which lives of course 
on our CryptoTransport layer, so if we get a response, yay we have a peer, and we use the
callback we passed to send_raw (which compresses the message, but funnilly i don't 
see where it decompresses message responses... (let me know if you know), here we,
it seems we do something that should be done within the dht, I see the manipulation
of the internals of the dht from here, this looks like it could be moved back to the dht

```
                    # Add this peer to active peers list
                    for idx, peer in enumerate(self._transport._dht._activePeers):
                        if peer._guid == guid or peer._address == address:
                            self._transport._dht._activePeers[idx] = self
                            self._transport._dht.add_peer(self._address, self._pub, self._guid, self._nickname)
                            return
```
maybe inside the method dht.add_peer() looks like this logic belongs, 
taking care of active peers... updating the routing table (which i see 
is where all the kademlia logic begins).

I'd break this constructor down, into just the contruction of the object and then
I'd have another method where the handshake happens, could be a start(), 
or start_handshake() method, which then gets called explicitly. 
This way it helps maintainers having to read less code (IMO), and allows for reusability of logic.

MarketApplication also happens to define what look like URL Request 
handlers in its constructor...
for the root "/", and /main/ a so called "MainHandler", then one for 
static files, and one for websocket, everything cool so far. 
(no clue what that post_joined() will be for, and I see I added my upnp
stuff here, ha... this will so not be here I think.)

In the end the tornado framework inherited __init__ method is called and
therefore and the handlers are passed to it. The transport wasn't... 
hmm, I keep wondering still if this is the place for transport to be defined,
and who does MarketApplication need to talk to that it defines a getter for it... 
(if it's only one object, then we probably shouldn't make transport
an attribute of ApplicationMarket (just thinking)

Let's go to CryptoTransportLayer, the interesting one that ends up having
something to do with p2p from what I read.

(first off, maybe the file where it resides should be renamed 
from "crypto2crypto.py" to something more adecuate like "transport.py", 
I suggest this because I was instinctively looking for a transport 
module where the ***TransportLayer classes where defined.)

So we quickly have to jump up CryptoTransportLayer > TransportLayer

and transportlayer doesn't live here... lives in p2p, it has pretty 
important methods to add_callbacks and trigger_callbacks, and it has...
has a listen() method. (so now I see that MarketApplication must be
listening for just HTTP with tornado, and here's our ear to the p2p world.)

(now I'm thinking that CryptoTransportLayer perhaps should be here on 
the "p2p" package (this would be that "transport" module I was looking for)

so listening consists of opening a zmq socket, not sure what 
IOLoop.current() means, I suppose it has to do with the internals 
of zmq to handle multiple connections (asynchronously or with threads, 
I still have to read about zmq) and right before it finishes we 
tell it what the hadling callback should do ("handle_recv") and 
there we receive a message, which passes through on_raw_message 
(let's see if it does some kind of validation of the data that just came), 
ok, does a try catch when it basically json-deserializes it 
(perhaps here we ought-to consider "MessageDecoder" abstraction that 
allows us to talk in different forms, one would be JSON, another some
more efficient binary package format that happens to fit very well 
in a UDP package and which would save us terabits per second if we
ever happen to be running a multimillionaire-node network, you will
want bandwidth, always.)

there's some special treatment here, I'd say very premature as the 
message handling logic gets split, perhaps we out to just not call
_init_peer("type") here, instead let's leave all the message handling
to "on_message". Let's make methods just do what they say, "on_raw_message"
just just convert from an expected raw format, into a json format for now
(since everything else has been wired with it), I'd would just convert here 
straight into a Universal, Class based Message Object
(from which we'll have a hierarchy of messages) so that a lot of
code can be reused and we're not constantly dealing with json dicts, 
and dicts and making typos on keys the interpreter can't help us tell 
us there's an error.

This way, also, we'll be able to handle as many message encoding 
mechanisms as we can, and now I'm thinking maybe this would also 
enable us to encrypt/decrypt messages, so a decryption element in
the message handling pipeline could be added (pipelines are cool 
abtractions for these things btw)

so let's see _on_message, so if msg type is not 'ok' we trigger the 
_callbacks, ok, that was brief (so yes, we better add that on 
hello_request handling here, which calls _init_peer(),

init peer ads a PeerConnection object to what seems an infinite list..., 
no capping of peers? easy to DDOS, or more likely to slowly get full of
connections... (that sounds familiar...) (oh oh)

what's "PeerConnection()", let's see, so it knows about the
CryptoTransporLayer (aka "transport" everywhere) it creates a logger, 
seems like there can be so many logger objects, but for this class 
it's just one... and it has a zmq Context(), but maybe this in 
practice is a CryptoPeerConnection which is where I see you decided 
to handle the encryption, at the connection level, per peer. hmm.

so it's been added to the list but it hasn't connected to anybody yet, 
let's pop back on the stack

so were just handled all thos messages with _on_raw_message and now we send()
an ok, let's see what sending is, so here we see the first use of the dht
routing table which seems to know about our contacts... which look like 
equivalents to "PeerConnection" objects.

with the stream we created on transport.listen() we send() to the peer given
to us, and if not, we check our dht for "activePeer", which have a 
send(data, callback) methods, let's look if that's like PeerConnection..
. jump, so we managed to hack the dht object to keep references to 
these guys (or maybe it came that way... or you've been very busy, 
which you look like you have on the dht object.)

so eventually the peerconnection sends what looks like a zlib compressed 
message, and might invoke a callback if one was given to handle the 
immediate response to a message if it were necessary.

So... on the CryptoTransportLayer constructor, which I believe is the one used,
we add the callbacks trigger_callbacks ends up using, because 
I can't see anybody adding those callbacks, so this must be the one used.