Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

[WIP] Add GraphQL interface to Ethereum node data #972

Closed
wants to merge 21 commits into from

Conversation

voith
Copy link
Contributor

@voith voith commented Aug 25, 2019

What was wrong?

closes #302

How was it fixed?

This is some very dirty code. Please don't bother to review. I will ping here once this is ready for review. I'm submitting early to show that I'm actively working on this.

To-Do

  • Clean up commit history

Cute Animal Picture

put a cute animal picture link inside the parentheses

@@ -0,0 +1,95 @@
import json
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to get rid of this file. This was added for testing.

@voith
Copy link
Contributor Author

voith commented Aug 30, 2019

This PR is now functionally complete. However, there's a lot more work remaining before this is ready for reviewing. I'm adding a bunch of TODO's for myself here.


TODO

  • Address all TODO and FIXME comments
  • Add typing
  • Test linking of blocks, transactions and accounts.
  • create a PR in py-evm to add method get_transaction_output
  • fix wheels test(This PR did not break them, wheels tests are flaky).
  • Refactor common code used by json-rpc and graphql and add it to a common utility.
  • Discuss with the team if the graphql enpoint needs to be exposed via HTTP. (exposing via http has its merits. There's a nice browser plugin called grqphiql that can be used to query from the browser) Serve graphiql through the HttpServer.
  • plug in the ChainReplacementEvent event.
  • Discuss the inconsistencies discovered in the DataTypes described in the EIP.
  • implement all the remaining Scalar types to match the EIP.
  • Discuss which part of the EIP couldn't be implemented because of the limitations in trinity's API like getLogs, FIlters, etc

@voith
Copy link
Contributor Author

voith commented Sep 18, 2019

Sorry, for keeping this PR on hold. Some work came up along the way. Will resume this on this weekend.

return hex(self.value)

async def resolve_index(self, info):
# FIXME: Figure out a way to get this value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a good starting point:

https://github.com/ethereum/py-evm/blob/1254611c2d622150e55c15148ced433ed153681c/eth/db/chain.py#L386-L401

We store this additional metadata about transactions. Maybe we need to store more?

@adamschmideg
Copy link

@voith Can you list

  • the inconsistencies discovered in the DataTypes described in the EIP
  • which part of the EIP couldn't be implemented because of the limitations in trinity's API (other than logs and filtering)

@voith
Copy link
Contributor Author

voith commented Oct 30, 2019

@adamschmideg My humble Apologies for the late reply. I will start working on this PR again from tomorrow. Just to warm things up, I fixed the broken eth_syncing endpoint which I will use in this PR.
Regarding your questions, it's been a while since I worked on this. I need to refresh my memory. OI I will try to recollect tomorrow when I get started on this.

@adamschmideg
Copy link

I'd like to write a test script for Trinity, similar to this one. It has a number of assumptions:

  • The client has a docker image. This is check (I just don't understand why it's around 1 GB for Trinity)
  • The client can import blocks from a file. I understand Trinity can't do that now. Maybe you have a workaround.
  • The client can be started to serve graphql. I'm waiting for this. It would be nice to have it even if the only query it serves is query {syncing}.

@cburgdorf
Copy link
Contributor

The client can import blocks from a file

@adamschmideg I don't have a workaround at hand but maybe I could find time to add that feature next week.

make lint happy

fix tests

fix tests

fix tests

fix tests

make lint happy
@voith voith changed the title [DIRTY][WIP] Add GraphQL interface to Ethereum node data [WIP] Add GraphQL interface to Ethereum node data Nov 9, 2019
@voith
Copy link
Contributor Author

voith commented Nov 10, 2019

The client can be started to serve graphql. I'm waiting for this. It would be nice to have it even if the only query it serves is

@adamschmideg Can you try the following.

  • Install trinity from my branch:
    pip install "git+https://github.com/voith/trinity.git@voith/eip-1767#egg=trinity"
    It might take a while to install as it will pull all the fixtures.
  • Make sure that trinity is on the right path, ideally use a virtualenv.
    which trinity
  • start trinity with graphql and graphiql. graphiql will help you query through your browser.
    trinity --graphql --enable-graphiql
    wait for some time till trinity starts the graphql and graphiql server
  • open a browser and visit
    http://127.0.0.1:8547/
  • run any query
{
  syncing {
      currentBlock
  }
}

@adamschmideg
Copy link

I had to brew install gmp to make it run.

When I refresh the browser, I get this error

  ERROR  2019-11-11 13:51:02,599                  http  Unrecognized exception while executing RPC
Traceback (most recent call last):
  File "/tmp/voith-trinity/venv/lib/python3.7/site-packages/trinity/rpc/http.py", line 41, in handler
    result = await rpc.execute_post(body_json)
  File "/tmp/voith-trinity/venv/lib/python3.7/site-packages/trinity/rpc/graph_ql/server.py", line 44, in execute_post
    return_promise=True
  File "/tmp/voith-trinity/venv/lib/python3.7/site-packages/promise/iterate_promise.py", line 10, in iterate_promise
    yield from promise.future  # type: ignore
concurrent.futures._base.CancelledError

The graphiql part is working (schema, etc), but running a { syncing { currentBlock }} just keeps waiting without returning any data. It synced a few thousand headers. It works for { gasPrice }, though. So in conclusion: yey 🎉

@voith
Copy link
Contributor Author

voith commented Nov 11, 2019

I've never seen the CancelledError before. The request was canceled for some reason? IDK.

The graphiql part is working (schema, etc), but running a { syncing { currentBlock }} just keeps waiting without returning any data.

Interesting! It takes a while for that endpoint to respond initially. The syncing endpoint was fixed recently and works great for me on this brach.
I will return None when it's not syncing(just like geth). But if it just hangs then it could be some other bug. Anyway, that is a separate issue and needs to be tackled in a different. This PR only does the necessary integration.

@pipermerriam Although this event_bus approach looks cool and helps build some features at ease, it's just too slow. It takes about 3 seconds sometimes to get a response from the eth_syncing endpoint. At least locally, I'd expect it to respond very soon.

@cburgdorf cburgdorf mentioned this pull request Nov 12, 2019
3 tasks
@cburgdorf
Copy link
Contributor

@adamschmideg I still need to write some tests but other than that the import and export commands are ready in this PR now #1266

They do only allow exporting / importing of single blocks so far (geth allows ranges)

@cburgdorf
Copy link
Contributor

t takes about 3 seconds sometimes to get a response from the eth_syncing endpoint. At least locally, I'd expect it to respond very soon.

Our JSON-RPC requests being served slowly and often timing out is a real problem that is also well reflected in our flaky integration tests. However, I don't the eventbus is at fault. We run benchmarks for the event bus and it can easily propagate thousands of events per second.

What I think is at fault is that our networking process may be overloaded with blocking calls, stalling the event loop. For these tests where you just import blocks and then query them through graphql one thing that you could try is to drastically trim down the jobs of the node. Maybe try running with --sync-mode none --disable-discovery --disable-upnp.

@voith
Copy link
Contributor Author

voith commented Oct 12, 2020

Sorry, I no longer have the bandwidth to work on this. So, I'm closing this PR

@voith voith closed this Oct 12, 2020
@voith voith deleted the voith/eip-1767 branch October 12, 2020 04:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support EIP1767 - GraphQL API for querying chain data
4 participants