omgnetwork / plasma-cash

122 stars 34 forks source link

operator-client bi-direction event system in WebSocket #68

Closed zhouer closed 6 years ago

zhouer commented 6 years ago

When sending transactions, the receiver client needs readltime notifications because it needs to confirm that the history has been received. And due to the clients may not always have public ip availability, it's easier to set a persistent connection between the server and a client, and let the server push notifications to clients.

Now, the child_chain_client connects to the server in its constructor, and provides emit, on event system for easier communications. Also, the client can register its identity, like address, after connecting to the server, and the server can identify each client to send or relay messages. For example:

Alice:

c = container.get_child_chain_client()
c.emit('join', 'alice')
c.on('relay', print)

Bob:

c = container.get_child_chain_client()
c.emit('join', 'bob')
c.emit('relay', {'dest': 'alice', 'message': 'hello'})

And due to a WebSocket framework was introduced on server side, the server startup command was changed to

python -m plasma_cash.child_chain

the orignal flask run command will failed to start the WebSocket service.

zhouer commented 6 years ago

Hi @bun919tw I fix the parentheses issue (and re-write the git history for some reasons), please review again, thanks!

zhouer commented 6 years ago

Flask-SocketIO got much more attentions, adoptions, and well maintained. SocketIO uses WebSocket as "one of its persistent connection solutions", but not uses WebSocket exclusively. To me, WebSocket is a W3C standard, but SocketIO is not a standard, and I preferred standardized protocols better. However, I can always switch to SocketIO, what do you think.

boolafish commented 6 years ago

TBH, I just prefer the code style of the lib more haha. I don't really mind what's beneath (as long as they are stable enough). But yeah, I don't think this should be blocker or what, it's more just a personal preference on the lib.

zhouer commented 6 years ago

By the way, I think supporting flask run is possible by just adding a wrapper.

zhouer commented 6 years ago

Flask-SocketIO overwrites flask command flask run https://github.com/miguelgrinberg/Flask-SocketIO/blob/master/setup.py#L27 https://dormousehole.readthedocs.io/en/latest/cli.html#id11

zhouer commented 6 years ago

Hi @boolafish

  1. The Flask-Sockets has been removed
  2. Operator server-side event handling is using SocketIO-style decorator functions
  3. child_chain_client unit tests for WebSocket related methods have been added
  4. flask run is still not supported
zhouer commented 6 years ago

I'm not sure what functions will be needed for implementing plasma-cash, functions currently supported are more like proof of concepts. I will leave the tests in the future PR.