Added WebsocketProvider

[voith]

What was wrong?

web3.py does not support websockets as discussed in #566

How was it fixed?

A WebsocketProvider has been implemented using websockets library.

TODO list:

• Address all todo comments.

• Fix integration tests

Cute Animal Picture

[voith]

Submitted early to get early feedback.
I have added the WebsocketProvider to the same tests that run integration tests for parity which earlier used only IPCProvider. The transactions tests will fail because the transactions will be repeated by both providers. However, other tests pass which proves that the code added works.

IMO, integration tests should test all providers. @pipermerriam If supporting all providers in the integration tests is the way to go then I'd be happy to add them in a separate PR.

[pipermerriam]

I agree with you with respect to running the integration tests using this new provider. I believe the appropriate way to do this would be to create both a test_parity_ipc.py and a test_parity_websocket.py test module which both did independent setup of the running parity process.

Note, that your should not simply copy/paste the current test_parity module as we don't want to duplicate that setup code, however we do want to ensure that it creates two separate independent parity_process fixtures for each test file. You may need to experiment a bit on how to do this, but I believe moving these fixtures into their own module and them importing them into each test file may work.

[pipermerriam]

cc @carver

I think this should be added as a core requirement that is installed by default.

[voith]

will do

[carver]

👍 core requirement seems fine here

[pipermerriam]

Ideally we want to have the life cycle of the thread and the provider be explicitly bound together such that if an instance of the provider goes away, so does the thread. This implies to me that we need both the thread and the loop from this function.

[voith]

I had taken the same approach before submitting, However I couldn't manage to close the event_loop ad the thread successfully. I tried doing this in __del__ method.
I admit that I have limited knowledge about the life cycle of python objects and threads. I will give this another try.
However, If you could give me some of guidance then It'd be very helfpful.

[pipermerriam]

nitpick

We've generally avoided single letter variables. Can you rename this to something like thread or loop_thread

[voith]

will do

[pipermerriam]

I would look into how the IPCProvider does it with the PersistantSocket object.

[voith]

will do

[voith]

@pipermerriam thanks for the review!
I will try to submit a PR with tests separated for ipc and http either tonight or tomorrow. This will help me add tests for the WebsocketProvider easily.

[voith]

depends on #710

[gsalgado]

Since the coroutines scheduled here (or indirectly by websockets) will be in a different thread than the one running the event loop, so I believe that has to be done using asyncio.run_coroutine_threadsafe(), no? At least that's what https://docs.python.org/3/library/asyncio-dev.html#concurrency-and-multithreading states

[voith]

@gsalgado I just checked the thread objects in each coroutine.
Since coro_make_request is scheduled by make_request using asyncio.run_coroutine_threadsafe(self.coro_make_request(request_data),WebsocketProvider._loop), the coroutines inside coro_make_request and the ones by websockets are scheduled in the same thread running the event loop.

However, Thanks for making me double check this!

[boneyard93501]

@voith i think with a very minor tweak, you'll eliminate a few issues. specifically, you may want to consider running one event loop, one thread only. you could easily get there switching from instance to class attributes and add little housekeeping. e.g., for a [quick poc] (https://gist.github.com/boneyard93501/b4aaf612513fb07ed5c4664eab6b2dd2)

now, i have encountered problems shutting the loop down gracefully, as you'll see in the comments, which means you get a bunch of exceptions when the daemonized thread force closes the loop. it's a common, well-known issue; still, it's ugly. maybe @gsalgado can have a look at the async_ws_thread_closer function.

[voith]

@boneyard93501 thanks for your code snippet! looking into it now.

[voith]

consider running one event loop, one thread only. you could easily get there switching from instance to class attributes and add little housekeeping. e.g

@boneyard93501 Although having one event loop was what I intended to implement, It was a bug in my code. Switching to ClassName to self did the trick. I had seen this problem before and I thought implementing a Singleton Eventloop class would do the trick. However, you saved me a lot of trouble :)

now, i have encountered problems shutting the loop down gracefully, as you'll see in the comments, which means you get a bunch of exceptions when the daemonized thread force closes the loop. it's a common, well-known issue; still, it's ugly

I have already tried steps taken by you to shut it down gracefully. you can have a look at code snippet

which means you get a bunch of exceptions when the daemonized thread force closes the loop

I agree and I know that its ugly but I couldn't find a better solution and atleast the user doesn't get to see the errors.

[voith]

@pipermerriam @carver @gsalgado I have addressed all your comments and I've added a bare minimum version of WebsocketProvider. Let me know if more needs to be done.

However, I won't be available until Monday in case more work needs to be done.

[voith]

Unrelated to this PR. But this fixture wasn't needed here

[voith]

Also tested:

In [10]: w = Web3(Web3.WebsocketProvider('wss://mainnet.infura.io/_ws'))

In [11]: w.eth.getBlock(0)
Out[11]: 
AttributeDict({'difficulty': 17179869184,
 'extraData': HexBytes('0x11bbe8db4e347b4e8c937c1c8370e4b5ed33adb3db69cbdb7a38e1e50b1b82fa'),
 'gasLimit': 5000,
 'gasUsed': 0,
 'hash': HexBytes('0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3'),
 'logsBloom': HexBytes('0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000'),
 'miner': '0x0000000000000000000000000000000000000000',
 'mixHash': HexBytes('0x0000000000000000000000000000000000000000000000000000000000000000'),
 'nonce': HexBytes('0x0000000000000042'),
 'number': 0,
 'parentHash': HexBytes('0x0000000000000000000000000000000000000000000000000000000000000000'),
 'receiptsRoot': HexBytes('0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421'),
 'sha3Uncles': HexBytes('0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347'),
 'size': 540,
 'stateRoot': HexBytes('0xd7f8974fb5ac78d9ac099b9ad5018bedc2ce0a72dad1827a1709da30580f0544'),
 'timestamp': 0,
 'totalDifficulty': 17179869184,
 'transactions': [],
 'transactionsRoot': HexBytes('0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421'),
 'uncles': []})

[pipermerriam]

I pushed a few commits to do final cleanup. This looks great. Solid work and a solid foundation for the new web3.async module that we're planning to build.

[pipermerriam]

@carver leaving this for you to make final review/merge/whatever

[boneyard93501]

@void4
re graceful shutdown: i was rummaging around aiohttp and uvloop to see if i could find anything better. no dice. not saying it's not there just that i couldn't make it work.
re documentation: as i was looking through uvloop, i stumbled upon some multiprocessing issues which reminded me of a somewhat heated discussion a couple years ago. upshot is, with the threading design at hand, the GIL would spit the bit, i.e. deadlock, if one tried to use wsprovider in processes. hence, you probably should add a big "this won't work with multiprocessing" warning to the very top of the documentation. if you're bored, curious, or both and enjoy a good GIL debate, have a look at this

[voith]

@pipermerriam Thanks for clean up. I have pushed a commit to fix the linting issues.
However, I have no idea why the tests are failing now. The failing tests seem to be completely unrelated. I tried rebasing with master, but tests are still failing.
cc @carver.

@boneyard93501 I will write some code in order to fully understand what you're trying to say and then I'll get back to you.

[voith]

ethereum/py-evm#483 might be cause for the failing tests, not sure though!

[carver]

@voith can you upgrade this PR to eth-tester 0.1.0-beta.22? I believe that should fix at least one of the problems: The older eth-tester version had a bad py-evm dependency.

[carver]

Ok, looks like everything is going to pass. The commit history is pretty messy right now. Are interested in squashing the fixups into a few cohesive commits? If not, I'm happy to just do a squash into a single commit.

[voith]

@carver feel free to squash it into a single commit!

[voith]

thanks everyone for helping me complete this.
I will submit work for the bounty as soon as gitcoin approves my request for faucet.

@boneyard93501 I'm sorry that I didn't address your comments but If anyone complains about the problems that you've forecasted, I'll resolve them myself!