Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraphQL API #17903

Closed
wants to merge 38 commits into from
Closed

GraphQL API #17903

wants to merge 38 commits into from

Conversation

Arachnid
Copy link
Contributor

@Arachnid Arachnid commented Oct 13, 2018

This PR defines a GraphQL API that implements the bulk of the functionality of the eth_ namespace in the JSON-RPC API, but with more friendly semantics and more efficient execution. In a test on my laptop, fetching all receipts for a recent block took 500 milliseconds with 32 threads with the existing JSON-RPC based API, but only 18ms with this GraphQL implementation.

I'm hoping to define this graphql schema as an ERC standard and get it adopted across a broad range of clients as a new standard API. This PR is intended to be a PoC and eventually a reference implementation.

After patching this PR in, you can try it out by running geth with the --graphql flag, and visiting localhost:8547 in your browser, which will give you an interactive GraphiQL query explorer, with inline help. For instance, try this query to get information on the latest synced block:

{
  block {
    number
    hash
    parent {
      hash
    }
    nonce
    transactionsRoot
    transactionCount
    stateRoot
    receiptsRoot
    miner {
      address
      balance
    }
    extraData
    gasLimit
    gasUsed
    timestamp
    logsBloom
    mixHash
    difficulty
    totalDifficulty
    ommerCount
    ommers {
      number
      hash
    }
    transactions {
      hash
      from {
        address
      }
      to {
        address
      }
    }
  }
}

The following table maps existing RPC calls under the eth_ namespace to their GraphQL equivalents:

RPC Status Description
eth_blockNumber IMPLEMENTED { block { number } }
eth_call IMPLEMENTED { call(data: { to: "0x...", data: "0x..." }) { data status gasUsed } }
eth_estimateGas IMPLEMENTED { estimateGas(data: { to: "0x...", data: "0x..." }) }
eth_gasPrice IMPLEMENTED { gasPrice }
eth_getBalance IMPLEMENTED { account(address: "0x...") { balance } }
eth_getBlockByHash IMPLEMENTED { block(hash: "0x...") { ... } }
eth_getBlockByNumber IMPLEMENTED { block(number: 123) { ... } }
eth_getBlockTransactionCountByHash IMPLEMENTED { block(hash: "0x...") { transactionCount } }
eth_getBlockTransactionCountByNumber IMPLEMENTED { block(number: x) { transactionCounnt } }
eth_getCode IMPLEMENTED { account(address: "0x...") { code } }
eth_getLogs IMPLEMENTED { logs(filter: { ... }) { ... } } or { block(...) { logs(filter: { ... }) { ... } } }
eth_getStorageAt IMPLEMENTED { account(address: "0x...") { storage(slot: "0x...") } }
eth_getTransactionByBlockHashAndIndex IMPLEMENTED { block(hash: "0x...") { transactionAt(index: x) { ... } } }
eth_getTransactionByBlockNumberAndIndex IMPLEMENTED { block(number: n) { transactionAt(index: x) { ... } } }
eth_getTransactionByHash IMPLEMENTED { transaction(hash: "0x...") { ... } }
eth_getTransactionCount IMPLEMENTED { account(address: "0x...") { transactionCount } }
eth_getTransactionReceipt IMPLEMENTED { transaction(hash: "0x...") { ... } }
eth_getUncleByBlockHashAndIndex IMPLEMENTED { block(hash: "0x...") { ommerAt(index: x) { ... } } }
eth_getUncleByBlockNumberAndIndex IMPLEMENTED { block(number: n) { ommerAt(index: x) { ... } } }
eth_getUncleCountByBlockHash IMPLEMENTED { block(hash: "0x...") { ommerCount } }
eth_getUncleCountByBlockNumber IMPLEMENTED { block(number: x) { ommerCount } }
eth_protocolVersion IMPLEMENTED { protocolVersion }
eth_sendRawTransaction IMPLEMENTED mutation { sendRawTransaction(data: data) }
eth_syncing IMPLEMENTED { syncing { ... } }
eth_getCompilers NOT IMPLEMENTED Compiler functionality is deprecated in JSON-RPC.
eth_compileLLL NOT IMPLEMENTED Compiler functionality is deprecated in JSON-RPC.
eth_compileSolidity NOT IMPLEMENTED Compiler functionality is deprecated in JSON-RPC.
eth_compileSerpent NOT IMPLEMENTED Compiler functionality is deprecated in JSON-RPC.
eth_newFilter NOT IMPLEMENTED Filter functionality may be specified in a future EIP.
eth_newBlockFilter NOT IMPLEMENTED Filter functionality may be specified in a future EIP.
eth_newPendingTransactionFilter NOT IMPLEMENTED Filter functionality may be specified in a future EIP.
eth_uninstallFilter NOT IMPLEMENTED Filter functionality may be specified in a future EIP.
eth_getFilterChanges NOT IMPLEMENTED Filter functionality may be specified in a future EIP.
eth_getFilterLogs NOT IMPLEMENTED Filter functionality may be specified in a future EIP.
eth_accounts NOT IMPLEMENTED Accounts functionality is not part of the core node API.
eth_sign NOT IMPLEMENTED Accounts functionality is not part of the core node API.
eth_sendTransaction NOT IMPLEMENTED Accounts functionality is not part of the core node API.
eth_coinbase NOT IMPLEMENTED Mining functionality to be defined separately.
eth_getWork NOT IMPLEMENTED Mining functionality to be defined separately.
eth_hashRate NOT IMPLEMENTED Mining functionality to be defined separately.
eth_mining NOT IMPLEMENTED Mining functionality to be defined separately.
eth_submitHashrate NOT IMPLEMENTED Mining functionality to be defined separately.
eth_submitWork NOT IMPLEMENTED Mining functionality to be defined separately.

@Arachnid Arachnid changed the title [WIP] GraphQL API GraphQL API Oct 18, 2018
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

Some minor nitpicks, otherwise I think this will make a great addition!

case int32:
*b = Uint64(input)
default:
err = fmt.Errorf("Unexpected type for BigInt: %v", input)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
err = fmt.Errorf("Unexpected type for BigInt: %v", input)
err = fmt.Errorf("Unexpected type for Uint64: %v", input)

Copy link
Contributor

Choose a reason for hiding this comment

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

Or Long ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it should be Long. Fixed.

common/types.go Outdated Show resolved Hide resolved
ethgraphql/main.go Outdated Show resolved Hide resolved
ethgraphql/main.go Outdated Show resolved Hide resolved
internal/ethapi/api.go Show resolved Hide resolved
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@raulk raulk left a comment

Choose a reason for hiding this comment

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

I've been thinking about a Pagination model in the context of EthQL. The complete connection model approach has one too many levels of depth for my taste. And we don't need to interoperate with Relay. But it serves as an inspiration.

Paged and non-paged query scenarios can co-exist, as both are desirable depending on the situation. User-driven introspective queries, analytics, diagnostics, etc. will prefer one-shot queries. Whereas Dapps, scripts, explorers, etc. should use pagination. We rely on the developer/user to decide responsibly (and infrastructure providers could block non-paged queries).

The fields that would benefit from pagination are:

  • Query->blocks
  • Query->logs
  • Block->transactions
  • Block->logs

Opaque cursors are useful. It allows clients like Geth to encode the tracking data in the cursor itself (e.g. base64 json/protobuf) and thus avoid storing server-side state.

Schema-wise, it could look like this:

input Selector {
  offset: Int
  first: Int  
  after: String
}

input PageInfo {
  cursor: String
  more: Boolean
}

type PagedBlocks {
  data: [Block!]!
  pageInfo: PageInfo!
}

type PagedLogs {
  data: [Log!]!
  pageInfo: PageInfo!
}

type NonPagedQueries {
  blocks(from: Long!, to: Long): [Block!]!
  logs(filter: FilterCriteria!): [Log!]!
}

type Query {
  blocks(from: Long!, to: Long, selector: Selector): PagedBlocks!
  logs(filter: FilterCriteria!, selector: Selector): PagedLogs!
 
  "Namespace for non-paged queries"
  nonPaged: NonPagedQueries!
}

Just some initial thoughts.

ommerHash: Bytes32!
# Transactions is a list of transactions associated with this block. If
# transactions are unavailable for this block, this field will be null.
transactions: [Transaction!]
Copy link
Contributor

Choose a reason for hiding this comment

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

Support for filtering transactions in different forms is something that EthQL users appreciate. Currently we have a split brain, where some filters take the form of fields (e.g. transactionsInvolving, transactionsRoles) and others are in an input type:

"""
A filter for transactions. Setting multiple criteria is equivalent to combining them with an AND operator.
"""
input TransactionFilter {
  "Only selects transactions that emit logs."
  withLogs: Boolean

  "Only selects transactions that received input data."
  withInput: Boolean

  "Only selects transactions that created a contract."
  contractCreation: Boolean
}

Conflating all filters into an input type that can be passed as an arg to the transactions field would be welcome, e.g. (some docs omitted for brevity; a catch-all type that can be structured for better semantics; simple on purpose):

input TransactionFilter {
  "Accounts from which a transaction was sent."
  from: [Address]
  "Accounts to which the transaction was sent."
  to: [Address]
  "Accounts that can appear in any side of the transaction."
  actor: [Address]

  withLogs: Boolean
  withInput: Boolean
  contractCreation: Boolean
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I'd like to leave that to a second PR as well, in order to simplify matters.

@Arachnid
Copy link
Contributor Author

@raulk Regarding pagination, I don't think it's necessary for blocks -> transactions, blocks -> logs, or query -> blocks - all of these are either self-limited by gas limit or by the user parameters (in the case of query -> blocks). I agree we should implement pagination for querying logs - I left it out of this PR because it would involve a deeper change in Geth's log filtering internals, and wanted to give that its own PR.

cfg.GraphQLHost = ctx.GlobalString(GraphQLListenAddrFlag.Name)
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Please remove this empty line, make code more compact and doesn't really provide any value.

Name: "graphqlport",
Usage: "GraphQL server listening port",
Value: node.DefaultGraphQLPort,
}
Copy link
Member

Choose a reason for hiding this comment

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

I've been working on reformatting the flags a bit for a while now, making them more namespace-y. E.g.

  --mine                         Enable mining
  --miner.threads value          Number of CPU threads to use for mining (default: 0)
  --miner.notify value           Comma separated HTTP URL list to notify of new work packages
  --miner.gasprice "1000000000"  Minimum gas price for mining a transaction
  --miner.gastarget value        Target gas floor for mined blocks (default: 8000000)

Please use the same format for any new flag: graphql.addr, graphql.port

Copy link
Member

Choose a reason for hiding this comment

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

Also, instead of piggiebacking on RPCCors and RPCVhosts, please define a separate set. Since we're talking about two different TCP endpoints, it's imho necessary to be able to configure them separately.

@@ -72,6 +72,23 @@ func (b Bytes) String() string {
return Encode(b)
}

func (b Bytes) ImplementsGraphQLType(name string) bool { return name == "Bytes" }
Copy link
Member

Choose a reason for hiding this comment

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

This is a public method. Please add a method doc for it.

@@ -72,6 +72,23 @@ func (b Bytes) String() string {
return Encode(b)
}

func (b Bytes) ImplementsGraphQLType(name string) bool { return name == "Bytes" }

func (b *Bytes) UnmarshalGraphQL(input interface{}) error {
Copy link
Member

Choose a reason for hiding this comment

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

This is a public method. Please add a method doc for it.

@@ -187,6 +204,23 @@ func (b *Big) String() string {
return EncodeBig(b.ToInt())
}

func (b Big) ImplementsGraphQLType(name string) bool { return name == "BigInt" }
Copy link
Member

Choose a reason for hiding this comment

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

This is a public method. Please add a method doc for it.

@@ -0,0 +1,73 @@
package ethgraphql
Copy link
Member

Choose a reason for hiding this comment

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

Please add the copyright preamble.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is in fact copied from elsewhere, MIT licensed. I've added the MIT license preamble, which I believe is compatible with the GPL.

// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.

package ethgraphql
Copy link
Member

Choose a reason for hiding this comment

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

Being the main file, please add a package doc. I'd also prefer to call this file something other than main. Perhaps simply graphql?

@@ -0,0 +1,289 @@
package ethgraphql
Copy link
Member

Choose a reason for hiding this comment

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

Please add the copyright preamble.

@@ -0,0 +1,13 @@
package ethgraphql
Copy link
Member

Choose a reason for hiding this comment

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

Please add the copyright preamble. Please rename the file to graphql_test.go.

@@ -30,6 +30,7 @@ import (
"github.com/ethereum/go-ethereum/cmd/utils"
"github.com/ethereum/go-ethereum/dashboard"
"github.com/ethereum/go-ethereum/eth"
"github.com/ethereum/go-ethereum/ethgraphql"
Copy link
Member

Choose a reason for hiding this comment

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

Can we name the package simply graphql? I mean it's not like Geth is ever going to have some other graphql thing added to it any time soon.

@Arachnid
Copy link
Contributor Author

@karalabe PTAL.

Copy link
Member

@gballet gballet left a comment

Choose a reason for hiding this comment

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

I have a few extra stylistic comments and your branch PR also needs to be rebased. Otherwise, looks good to me!

receipts []*types.Receipt
}

// resolve returns the internal Block object represennting this block, fetching
Copy link
Member

Choose a reason for hiding this comment

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

typo: representing

common/types.go Show resolved Hide resolved
common/types.go Show resolved Hide resolved
common/hexutil/json.go Show resolved Hide resolved
common/hexutil/json.go Show resolved Hide resolved
common/hexutil/json.go Show resolved Hide resolved
graphql/graphiql.go Show resolved Hide resolved
@Arachnid
Copy link
Contributor Author

@gballet @karalabe I've fixed the typo, and @kshinn has rebased the PR here: #18445

@gballet
Copy link
Member

gballet commented Jan 17, 2019

Closing this PR and using #18445 instead

@gballet gballet closed this Jan 17, 2019
@holiman holiman added the todo-pr-into-docs This tag means that we should take the PR description and write docmentation based on it label Feb 8, 2019
@Arachnid
Copy link
Contributor Author

I've written up a draft EIP for standardising this functionality here: https://github.com/Arachnid/EIPs/blob/graphql/EIPS/eip-draft-graphql.md

@timbeiko
Copy link
Contributor

@Arachnid is the repo for your draft EIP private? The link seems broken.

@Arachnid
Copy link
Contributor Author

@timbeiko Sorry; it's since been merged as a draft, and can be found at https://eips.ethereum.org/EIPS/eip-1767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
todo-pr-into-docs This tag means that we should take the PR description and write docmentation based on it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants