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

[RFC] GraphQL API #1817

Open
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@Schmavery
Copy link
Contributor

Schmavery commented Mar 8, 2019

Describes the api we're envisioning for use in our wallet app.

Tiny crash course in GraphQL for any reviewers not familiar:

  • All types are nullable by default. You add a ! to the end to make a type non-nullable, so you'll see a lot of that.
  • An interaction with the server can be in 1 of 3 categories: Query, Mutation, or Subscription. A query reads data with no side-effects, a mutation updates the model, and subscriptions represent a stream of events of a certain type. The types Query, Mutation, and Subscription define the entry-points for queries, and then the other types define the shape of the objects those queries interact with.
  • balance(key: PublicKey!, consensus: ConsensusStatus): Int64! represents a field that accepts 2 arguments, one optional (consensus) and returns a non-null int.
  • All queries work by deciding which fields you want to receive. For example, this query would return all the amount and fee of all transactions for a given public key (here, the filterBySenderOrReceiver is an named argument, which is usually how this sort of filtering is implemented):
query transactionAmountAndFee($key: PublicKey) {
  transactions(filterBySenderOrReceiver: $key) {
    amount
    fee
  }
}

Please share any thoughts or questions :)

Edit: a cool tip from @sgrove:

npm install -g graphql-faker
graphql-faker --open

Then paste in the schema definition here and you'll be able to make fake queries against it.

Schmavery added some commits Mar 8, 2019

@Schmavery Schmavery requested review from bkase , es92 and nholland94 Mar 8, 2019

@jkrauska

This comment has been minimized.

Copy link
Contributor

jkrauska commented Mar 8, 2019

Can we add NetworkStatus? I'd like to be able to pull most of what comes from 'coda client status' via this api.

-----------------------------------
Global Number of Accounts:  20
Block Count:                116
Local Uptime:               4458s
Ledger Merkle Root:         292418354174033439743527567851845697026036314118203109056104590574079440142087535313704528
Staged-ledger Hash:         ((ledger_hash 292418354174033439743527567851845697026036314118203109056104590574079440142087535313704528)(aux_hash"\147\135q\029X\1449\022j\199/\174T:\173\135\135\224\191o\163\219\194\2525\217?\136\012O>!36"))
State Hash:                 284422071298342452487202553166820579017048582638283270132114319831729502432303167849786256
Git SHA1:                   137fbff011c1b0ee9fd15ef9b21a424f7bb38351
Configuration Directory:    /home/jkrauska/testfu/test-seed-1
Peers:                      Total: 6 (127.0.0.1:8433 127.0.0.1:8453 127.0.0.1:8423 127.0.0.1:8443 127.0.0.1:8503 127.0.0.1:8413)
User_commands Sent:         22
Snark Worker Running:       false
Proposer Running:           false
Best Tip Consensus Time:    742:74
Consensus Time Now:         742:75
Consensus Mechanism:        proof_of_stake
Consensus Configuration:
    delta = 3
    k = 6
    c = 8
    c_times_k = 48
    slots_per_epoch = 144
    slot_duration = 30000
    epoch_duration = 4320000
    acceptable_network_delay = 90000
@Schmavery

This comment has been minimized.

Copy link
Contributor Author

Schmavery commented Mar 8, 2019

Copying suggestions from @sgrove in discord:

  • Implement relay-style cursor-based pagination for payments https://facebook.github.io/relay/graphql/connections.htm
  • I might put this [version] under a object, e.g. codeNode: { version: String!} - that way it's extensible and self-descriptive
  • Also, be careful about adding a top node field, that should be reserved for another useful pattern. You don't have one now, but you have the concept of node in the protocol that's different from the graphql notion, so trying to head that off just in case
  • Every mutation should take custom type, Input, and return a custom object type, ResponsePayload. Justification: https://blog.apollographql.com/designing-graphql-mutations-e09de826ed97
  • Another thing to note is that you have a lot of required fields. You want to be really sure that those fields are never nullable. Because going non-nullable => nullable is considered a breaking change, and god willing, you'll never, ever push a breaking api change
@bkase

This comment has been minimized.

Copy link
Contributor

bkase commented Mar 8, 2019

This is great! I'll look more closely later, but off the top of my head, it looks like we don't discuss error handling. If we do what's proposed in that Apollo blog post, maybe we won't need to answer that question now.

@Schmavery

This comment has been minimized.

Copy link
Contributor Author

Schmavery commented Mar 9, 2019

@jkrauska good idea, I was thinking it might be good to combine a couple "status-y" queries (like network, stakingStatus) into bigger "nodeStatus" and "networkStatus" queries that returned something like this (and even just combine those two if we want):

type NodeStatus {
  stakerRunning: Boolean # or proposerRunning, whatever name we want I guess
  snarkWorker: SnarkWorker
  gitSHA1: String
  uptimeSecs: Int
}

type NetworkStatus {
  numberOfAccounts: Int
  blockCount: Int
  peers: [Peer]!
  consensusConfig: ConsensusConfig!
}

type Peer {
  ipAddress: String!
}

type ConsensusConfig {
 mechanism: MechanismsEnum!
 delta: Int
 k: Int
 etc
}

Not sure how much we'll need most of those hashes (or consensus time?) for the wallet, but if there's some compelling case to include them maybe we can still do that.

Schmavery added some commits Mar 9, 2019

}
type PaymentEdge {
cursor: String

This comment has been minimized.

@bkase

bkase Mar 11, 2019

Contributor

Should this be an opaque scalar type instead of a string as well?

This comment has been minimized.

@Schmavery

Schmavery Mar 12, 2019

Author Contributor

I made a note in the RFC about how we should probably be avoiding custom scalars in general, but for this specific case, it might be ok since none of the clients will need to handle it directly (especially if it serializes as a string)

objects, but if we could create one by hashing together enough properties of the
objects, it might help with this and avoid accidents comparing fields in an insufficient way.
- Authentication
- Potentially out of scope: How will this evolve and be used in the future, when most wallets will just run a light node locally?

This comment has been minimized.

@bkase

bkase Mar 11, 2019

Contributor

It may be worth addressing this a bit: Will we want our own layer in front of graphql for a full node, and in front of something else when the light node is in the browser? Is it easy to run a graphql server from a web worker? Does this already exist? Would this be the right interface for light nodes?

We don't have to answer all of these questions now, but if it seems like we won't expose a graphql interface for light nodes, it may be worth pushing through a "suboptimal" graphql API faster given that most API consumers wouldn't see the graphql layer directly. If it seems like we would expose a graphql layer for the light-node version, maybe we do want to be more careful for the bits that are shared between light/full nodes.

You could also make the argument that if we have to consider light/full nodes anyway we may need to do a larger redesign of the API -- and given that we are only thinking about the first version of the wallet, maybe we don't want to deal with this extra complexity right now.

This comment has been minimized.

@Schmavery

Schmavery Mar 12, 2019

Author Contributor

I'd think we will likely always want an api for accessing full nodes (in many cases remotely) even when we do have light nodes embedded in the wallet. I wouldn't be surprised if even in the case where you have the option of a light node locally while being connected to a full node, you would just rely on the full node (since you wouldn't need to make queries to the network about the current state of the ledger etc).

On the webworker question, it seems possible, if not common. Apollo exposes something called "link" which lets you tell the client how to resolve the queries. Someone used this to build a prototype that talks to a graphql server in a webworker. There's also stuff like graphql-anywhere, which could be even more flexible.

The less explored territory is probably on the "server side", where we might need to write a simplified custom backend (in addition to the existing async and lwt ones) for ocaml-graphql-server that communicates within the browser. Of course, we would probably have to do something similar to this even if we didn't use graphql for in-browser communication.

@enolan
Copy link
Contributor

enolan left a comment

Can you also add an example of how pagination will work?

Show resolved Hide resolved rfcs/0000-graphql-api.md
}
type Payment {
nonce: Int!,

This comment has been minimized.

@enolan

enolan Mar 12, 2019

Contributor

GraphQL ints are signed 32 bit, Coda nonces are unsigned 32 bit.

This comment has been minimized.

@Schmavery

Schmavery Mar 13, 2019

Author Contributor

Good point on the signedness, we'll have to do whatever we do for the 64 bit ints, it seems (which is probably represent it with a custom scalar that serializes as a string, unfortunately)

Show resolved Hide resolved rfcs/0000-graphql-api.md Outdated
Show resolved Hide resolved rfcs/0000-graphql-api.md Outdated
Show resolved Hide resolved rfcs/0000-graphql-api.md Outdated
Show resolved Hide resolved rfcs/0000-graphql-api.md Outdated
# Gets balance of key at a certain consensus state
# Note: `consensus` is optional as we will likely decide one
# state to be the "real" balance (probably FINALIZED)

This comment has been minimized.

@enolan

enolan Mar 12, 2019

Contributor

Given 30 seconds slots and k=2160, finality takes between 18 and 144 hours (depending on slot fill rate) which is way too long. We're going to have to settle for probabilistic finality.

simpler/lighter full node, with the obvious cost that whenever you close your wallet app, you're missing
any transactions that might be happening, which isn't great for the wallet experience.
- The wallet could be responsible for storing the private keys as well, but then the node would have to ask the wallet
for them whenever it needed them for snarking etc. Probably not what we want.

This comment has been minimized.

@enolan

enolan Mar 12, 2019

Contributor

You need private keys for proposing and sending payments, but not for snarking. Sending a payment is usually interactive, and the vast majority of people will delegate rather than run a proposer themselves. In addition, we'll want to support hardware wallets in the future. If we think of wallets as strongly separate from nodes, then keys should be managed by the wallets. If every application that uses Coda packages a light client that it's tightly bound to (if the resource requirements are low enough for that to be reasonable), then we can store them wherever.

This comment has been minimized.

@bkase

bkase Mar 13, 2019

Contributor

Yes I think the plan is we'd always store on the node, but sometimes the node is a js_of_ocaml lightweight thing in a web worker. For the first version of the wallet (targeting pro users) they'll be running a full node.


- The payment and block history for the wallets could be only stored on the client. This would
make the node only be responsible for storing the private keys of the wallets. This results in a somewhat
simpler/lighter full node, with the obvious cost that whenever you close your wallet app, you're missing

This comment has been minimized.

@enolan

enolan Mar 12, 2019

Contributor

We should give the nodes a set of public keys to track, and not require that you have the corresponding private key.

Show resolved Hide resolved rfcs/0000-graphql-api.md Outdated
@jkrauska

This comment has been minimized.

Copy link
Contributor

jkrauska commented Mar 13, 2019

@jkrauska good idea, I was thinking it might be good to combine a couple "status-y" queries (like network, stakingStatus) into bigger "nodeStatus" and "networkStatus" queries that returned something like this (and even just combine those two if we want):

type NodeStatus {
  stakerRunning: Boolean # or proposerRunning, whatever name we want I guess
  snarkWorker: SnarkWorker
  gitSHA1: String
  uptimeSecs: Int
}

type NetworkStatus {
  numberOfAccounts: Int
  blockCount: Int
  peers: [Peer]!
  consensusConfig: ConsensusConfig!
}

type Peer {
  ipAddress: String!
}

type ConsensusConfig {
 mechanism: MechanismsEnum!
 delta: Int
 k: Int
 etc
}

Not sure how much we'll need most of those hashes (or consensus time?) for the wallet, but if there's some compelling case to include them maybe we can still do that.

The ledger hash is pretty fundamental for identifying forks.

I'd thinking of the graphql api as a potential replacement for all RPC calls from client to daemon, so it would be good to keep coverage. (allowing anyone to re-skin a cli/tui/gui an interface to the daemon)

@jkrauska

This comment has been minimized.

Copy link
Contributor

jkrauska commented Mar 13, 2019

can you add schema for payment receipts? (sorry if I missed it)

each payment should return one, and there should be a query method to confirm after-the-fact that a payment has succeeded.

input AddWalletInput {
public: PublicKey
private: PrivateKey

This comment has been minimized.

@johnwu93

johnwu93 Mar 13, 2019

Contributor

Can't you derive the PublicKey with the PrivateKey?

This comment has been minimized.

@Schmavery

Schmavery Mar 13, 2019

Author Contributor

I didn't know if that was a given, but if it is then we could remove the public key

ERROR
BOOTSTRAP # Resyncing
CATCHUP # Synced but a little out
STALE # You haven't seen any activity recently

This comment has been minimized.

@johnwu93

johnwu93 Mar 14, 2019

Contributor

How exactly would you define STALE? How long would do you not have to receive transitions in order to be considered stale?

This comment has been minimized.

@Schmavery

Schmavery Mar 14, 2019

Author Contributor

We don't have a strong definition of this yet, we might need to see what the value is experimentally? Or calculate some statistically-reasonable fraction of k?

Show resolved Hide resolved rfcs/0000-graphql-api.md Outdated
@Schmavery

This comment has been minimized.

Copy link
Contributor Author

Schmavery commented Mar 15, 2019

can you add schema for payment receipts? (sorry if I missed it)

@jkrauska I'm not sure what a payment receipt looks like but I think we should be able to add anything we need for that to the Payment type. This RFC proposes we also add identifiers to payments (or transactions in general), one of the reasons being that then we can easily add a query to get the status of a specific one (in addition to helping resolve updates that are being streamed to the client).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.